Skip to content

Use raw string for docstrings containing escape sequences #455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 20, 2022
Merged

Use raw string for docstrings containing escape sequences #455

merged 1 commit into from
Jun 20, 2022

Conversation

kmaehashi
Copy link
Contributor

Hello, my first pull-request here. 😃
When running array-api-tests against CuPy, I experienced the following error:

+ ARRAY_API_TESTS_MODULE=cupy.array_api
+ pytest
ImportError while loading conftest '/tmp/tmp.IchRO6sbmt/array-api-tests/conftest.py'.
conftest.py:7: in <module>
    from array_api_tests import _array_module as xp
array_api_tests/__init__.py:6: in <module>
    from ._array_module import mod as _xp
array_api_tests/_array_module.py:4: in <module>
    from . import stubs
array_api_tests/stubs.py:30: in <module>
    name_to_mod[name] = import_module(f"signatures.{name}")
E     File "/tmp/tmp.IchRO6sbmt/array-api-tests/array-api/spec/API_specification/signatures/elementwise_functions.py", line 601
E       """
E       ^
E   SyntaxError: invalid escape sequence \s

This was because there are unescaped uses of \ in several docstrings of array-api code, and CuPy runs tests with error::DeprecationWarning warning filter which strictly checks the invalid escape sequences.

This PR fixes the issue by using raw string for such docstrings, in the same way as done in cholesky:

array-api/spec/API_specification/array_api/linalg.py

Lines 4 to 5 in 6a35bc1

def cholesky(x: array, /, *, upper: bool = False) -> array:
r"""

I confirmed there are no invalid escape sequences anymore:

find . -name "*.pyc" -delete
python -W error::DeprecationWarning -m compileall spec

Thanks.

honno
honno approved these changes Jun 14, 2022
View reviewed changes
Copy link
Member

@honno honno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep LGTM, thanks!

Note I won't be able to update array-api-tests yet to use this immediately, as this repos changed a fair bit in the last ~month and so I need to update a few things to support it again. Probably low-priority for now 😕

@leofang
Copy link
Contributor

leofang commented Jun 14, 2022

Right we fixed the signatures to make sure they can be referenced in Sphinx (#430). But, @honno why is it a low priority? It sounds like after the signature fix the array-api tests just cannot be run? Did it pin this repo as a submodule?

@honno
Copy link
Member

honno commented Jun 14, 2022 •
edited
Loading

Did it pin this repo as a submodule?

Yep.

Just hacking array-api-tests for this PR to be parsed I noticed a fair few spec changes that mess with prior assumptions for other parts of the test suite—I'll want to fix these properly, but it's not necessary for testing v1 Array API namespaces, and testing v2 namespaces isn't useful for at least another month. I'll definitely get round to it, it's just I have dataframe work which is high priority for at least the next 2 weeks.

@leofang
Copy link
Contributor

leofang commented Jun 14, 2022

We'll backport #430 to v1 so it's still needed, but I agree if array-api is pinned it's not urgent.

kgryte
kgryte approved these changes Jun 20, 2022
View reviewed changes
Copy link
Contributor

@kgryte kgryte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, @kmaehashi!

@kgryte kgryte added the Maintenance Bug fix, typo fix, or general maintenance. label Jun 20, 2022
@kgryte kgryte added this to the v2022 milestone Jun 20, 2022
@kgryte kgryte merged commit 42eb7a0 into data-apis:main Jun 20, 2022
@kmaehashi kmaehashi deleted the fix-escape-sequences branch June 20, 2022 03:13
@jakevdp
Copy link

jakevdp commented May 24, 2023 •
edited
Loading

I'm running into this issue when attempting to test against array-api@2022-12. Any suggestions for how to proceed? Is there any safe tag to checkout if I want to test version 2022-12?

$ Run pytest --ci array-api-tests/array_api_tests/
ImportError while loading conftest '/home/runner/work/jax/jax/array-api-tests/conftest.py'.
array-api-tests/conftest.py:9: in <module>
    from array_api_tests import _array_module as xp
array-api-tests/array_api_tests/__init__.py:8: in <module>
    from ._array_module import mod as _xp
array-api-tests/array_api_tests/_array_module.py:4: in <module>
    from . import stubs
array-api-tests/array_api_tests/stubs.py:31: in <module>
    name_to_mod[name] = import_module(f"signatures.{name}")
E     File "/home/runner/work/jax/jax/array-api-tests/array-api/spec/API_specification/signatures/linalg.py", line 44[7](https://github.com/google/jax/actions/runs/5073141741/jobs/9111776151?pr=16099#step:6:8)
E       """
E       ^^^
E   SyntaxError: invalid escape sequence '\*'

@jakevdp jakevdp mentioned this pull request May 24, 2023
Closed
@jakevdp
Copy link

jakevdp commented May 24, 2023

Opened an issue in #631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kgryte kgryte kgryte approved these changes

@honno honno honno approved these changes

Assignees
No one assigned
Labels
Maintenance Bug fix, typo fix, or general maintenance.
Projects
None yet
Milestone
v2022
Development

Successfully merging this pull request may close these issues.
5 participants
@kmaehashi @leofang @honno @jakevdp @kgryte